-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Don't ICE on operator trait methods with generic methods #104216
Don't ICE on operator trait methods with generic methods #104216
Conversation
r? @estebank (rustbot has picked a reviewer for you, use r? to override) |
cc @davidtwco, @compiler-errors, @JohnTitor, @estebank, @TaKO8Ki |
Emit a fatal error instead.
e5df0de
to
cedaaa6
Compare
assert_eq!(generics.params.len(), 0); | ||
|
||
if generics.params.len() != 0 { | ||
tcx.sess.emit_fatal(OpMethodGenericParams { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be fatal? Fatal errors should be reserved for particularly unrecoverable cases, and this one doesn't seem (at least initially) like one to me.
We could just return None
here -- what does that do to the error reporting?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With a basic emit_err
it causes further ICEs in other places. I haven't tried returning None
but I assume it's gonna emit further errors about things not being found.
This code path is only hit when using no_core
in weird ways (like by being a fuzzer) so I don't think we should care too much about recovery.
@compiler-errors is right that we should avoid fatal errors, but in the interest of removing the existing ICE and given that no_core isn't as common, I'll go ahead and merge. @bors r+ |
…r-traits, r=estebank Don't ICE on operator trait methods with generic methods Emit a fatal error instead. fixes rust-lang#104213
Rollup of 9 pull requests Successful merges: - rust-lang#100633 (Consider `#[must_use]` annotation on `async fn` as also affecting the `Future::Output`) - rust-lang#103445 (`#[test]`: Point at return type if `Termination` bound is unsatisfied) - rust-lang#103924 (Fix broken link in description of error code E0706) - rust-lang#104146 (Retry binding TCP Socket in remote-test-server) - rust-lang#104169 (Migrate `:target` rules to use CSS variables) - rust-lang#104202 (Fix ICE rust-lang#103748) - rust-lang#104216 (Don't ICE on operator trait methods with generic methods) - rust-lang#104217 (Display help message when fluent arg was referenced incorrectly) - rust-lang#104245 (Reduce default configuration's dependency upon static libstdcpp library (rust-lang#103606)) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Emit a fatal error instead.
fixes #104213